Conversation
cdce8p
added
Enhancement ✨
Pull Request Test Coverage Report for Build 3463412808
💛 - Coveralls |
cdce8p
force-pushed
the
try-node-breaking
branch
3 times, most recently
from
41538f0 to
9c9e249
Compare
cdce8p
force-pushed
the
try-node-breaking
branch
from
9c9e249 to
ac9f4c4
Compare
|
@cdce8p Should this be reviewed? Or should we wait until we are sure we can start merging |
The change itself is done. Might only need to resolve merge conflicts and update the changelog when we start with |
cdce8p
added
the
Needs review 🔍
cdce8p
force-pushed
the
try-node-breaking
branch
from
ac9f4c4 to
e189246
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1867 +/- ##
==========================================
- Coverage 92.84% 92.77% -0.07%
==========================================
Files 94 94
Lines 10964 10941 -23
==========================================
- Hits 10179 10151 -28
- Misses 785 790 +5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Pierre-Sassoulas
left a comment
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
It's not a work in progress anymore, right ? I think it's going to simplify a lot of checks in pylint 👍
cdce8p
force-pushed
the
try-node-breaking
branch
from
8a89565 to
2568e58
Compare
The PR is basically ready at this point. We could merge it as is. Once we merge it, I'll create another alpha release just for it. |
jacobtylerwalls
left a comment
jacobtylerwalls
left a comment
There was a problem hiding this comment.
Great! Eager to merge this after we release 3.0.0a7 and can devote an alpha release to just this change.
|
Let's do it ! :) |
5 participants
Description
Replaces both #1388 and #1389. I first tried implementing the migration plan laid out in #1388 (comment). However, the new
Trynode not being part of the "official" tree caused a lot of issues.parentattributes on all child nodes correctly. Since it needs to point to the immediate parent, it would have been different depending if a child was part of the "old"TryExceptor the newTrynode. That would basically mean that we couldn't reuse the child nodes and instead need to create new ones.try_nodeattribute.I reached the conclusion that it might be better to do a breaking change now, instead of trying to work around a solution which would likely create new issues and would still lead to a breaking change down the line.
--
This PR replaces the old
TryExceptandTryFinallynodes with the newTrynode. Python replaced the old nodes a long time ago in 3.3. The new node simplifies things a bit and even helps generate better line numbers for pylint errors in try-except blocks. It will also make it much easier to add support forTryStarnodes which would just need to inherit fromTry. pylint-dev/pylint#7703--
As this PR is a (major) breaking change, I suggest it should be the start for astroid
3.0once merged. I'm going to create a planning issue for3.0. However, even with the breaking change, the work required to update is manageable. I'll also open a pylint PR to update our code base.Trynode pylint#77673.0#1868--
What's left to do
I'll also mention it in the
3.0issue, but I think we should create a separate docs page just describing the breaking changes and how to update plugins. That can probably be done separately. If someone else would like to take the lead on this, I would appreciate any help. Some notesTryExceptandTryFinallyremovedvisit_*andleave_*methods